use chef handler to run inspec tests #113

vjeffrey · 2016-10-17T23:16:00Z

so many more things to do. just the beginning...working on viz integration parts now.

vjeffrey · 2016-10-18T12:23:27Z

successful converge with visibility with a change from:

# node.default['audit']['profiles'].merge!({
#   'nathen/tmp_compliance_profile' => {
#     'source' => "#{PROFILES_PATH}/tmp_compliance_profile"
#   }
# })

to
node.default['audit']['profiles'].push("#{PROFILES_PATH}/tmp_compliance_profile")

still need to modify to ensure json results get to the right place - right now just cli json output

Signed-off-by: Victoria Jeffrey <vjeffrey@chef.io>

vjeffrey · 2016-10-19T02:03:51Z

need to fix the one of the viz tests, but this works with chef visibility.

Signed-off-by: Victoria Jeffrey <vjeffrey@chef.io>

chris-rock · 2016-10-19T10:53:09Z

examples/kitchen/.kitchen.linux.yml

@@ -1,37 +0,0 @@
---


we should not delete the examples. Instead we should migrate to work with the new version

chris-rock · 2016-10-19T10:56:30Z

attributes/default.rb

@@ -46,11 +46,9 @@
 # fail converge after posting report if any audits have failed
 default['audit']['fail_if_any_audits_failed'] = false

-# inspec gem version to install(e.g. '1.1.0')
-default['audit']['inspec_version'] = '1.2.0'


we are going to need it for now

Signed-off-by: Christoph Hartmann <chris@lollyrock.com>

…t-2.0

chris-rock · 2016-10-19T12:07:54Z

recipes/default.rb

-    notifies :touch, "file[#{compliance_cache_directory}/#{p}]", :delayed
-    notifies :execute, "compliance_report[#{report_collector}]", :delayed
-  end
+chef_inspec 'inspec' do


chef_inspec feels like a strange name

chris-rock · 2016-10-19T12:08:57Z

resources/inspec.rb

@@ -1,7 +1,7 @@
 # encoding: utf-8

 provides :chef_inspec


can we just inspec for provides and as a resource name instead of chef_inspec?

of course! let's do it!

chris-rock · 2016-10-19T12:11:09Z

recipes/default.rb

-  quiet node['audit']['quiet'] unless node['audit']['quiet'].nil?
-  action :nothing
-end if node['audit']['profiles'].values.any?
+inspec_report 'report' do


Do you think we should combine:

chef_handler 'Chef::Handler::AuditReport' do source "#{handler_directory}/audit_report.rb" action :enable end inspec_report 'report' do action :execute end

how do we combine them?

seems like the right way to go, just unsure how?

chris-rock · 2016-10-19T12:37:40Z

recipes/default.rb

-    notifies :touch, "file[#{compliance_cache_directory}/#{p}]", :delayed
-    notifies :execute, "compliance_report[#{report_collector}]", :delayed
-  end
+chef_handler 'Chef::Handler::AuditReport' do


As discussed with @vjeffrey we try to find a way that does not require us to write a file on the host node

chris-rock · 2016-10-19T13:03:37Z

.kitchen.yml

@@ -64,8 +64,6 @@ suites:
    attributes:
      audit:
        profiles: &profiles


not 100% sure what &profiles should refer to

leftover from initial implementation.. Yaml syntax docs: "Repeated nodes are first identified by an anchor (marked with the ampersand - “&”), and are then aliased (referenced with an asterisk - “*”) thereafter."

jeremymv2 · 2016-10-19T15:55:41Z

attributes/default.rb

@@ -46,11 +48,9 @@
 # fail converge after posting report if any audits have failed
 default['audit']['fail_if_any_audits_failed'] = false



Since we're moving away from a resource converge I don't think 'fail_if_any_audits_failed' makes sense any more. The original intent was to fail during converge in a pipeline phase such as kitchen converge. Now, with the report handler, we'd rely on kitchen-inspec to reference the profiles and kitchen verify to provide a pass|fail status to the CI framework.

sounds good to me! i'll remove it

vjeffrey · 2016-10-19T16:21:04Z

still need to figure out the chef_handler putting a source file on the system thing (https://github.com/chef-cookbooks/audit/pull/113/files#diff-55bf87238c9b8af164c5133a60721f12R9)
I've asked around...ppl are unsure of what to do about that. one suggestion: "maybe the chef_handler resource can delete the file if you use the :disable action. "

jeremymv2 · 2016-10-19T17:45:41Z

Most handlers that I've used lay down the handler libs as a versioned gem. One of the better examples is the DataDog handler: https://github.com/DataDog/chef-datadog/blob/master/recipes/dd-handler.rb#L32-L38

vjeffrey · 2016-10-19T20:05:27Z

yup. so we were trying to avoid the gem thing in the case of an air-gapped env/to avoid outside deps (I know that doesn't make much sense right now since we are installing inspec gem but we are trying to find a way to move away from that and vendor) --- I spoke with joshua timberman and jj ashgar and both said ya, those are the only two ways of doing the chef handler thing, so i'm just gonna whip up a quick delete directory cleanup recipe and include it at the end of default with a good ol include_recipe call. coming soon! just have to make food for ppl real quick, then i'll do it :)

jeremymv2 · 2016-10-19T22:40:30Z

recipes/clean-up.rb

+directory handler_directory do
+  recursive true
+  action :delete
+end


Oh, I see what you're achieving now 😃 more clearly. However, I suppose I don't understand what the requirement is to cause us to remove the chef handler ruby library code.
To me it smells fishy because it seems like it's akin to running an app that deletes all traces of itself after execution. I can think of two potential concerns that arise from that:

It's not in the spirit of transparency - we're running code on nodes and then deleting that code from the nodes.

Deleting the handler code from the node could make troubleshooting harder since it requires version correlation against a git repo or chef server to determine what actual code was running when the handler fired. Having the code intact on the node gives you a 100% assurance that you know what executed.

If this moves forward, I recommend adding the standard header to the recipe which gets generated from chef generate recipe clean-up One last thing on this commit, just me perhaps, but using - in recipe names is not consistent with the requirement that cookbooks are not allowed to have - in the name 😄

# # Cookbook Name:: audit # Recipe:: clean-up # # Copyright (c) 2016 The Authors, All Rights Reserved.

@jeremymv2 thanks for the honest feedback. That is very helpful! Background: This was an approach @vjeffrey and I discussed before. From my perspective it feels strange to install code on a node just for runtime and not cleaning the tmp stuff up. Nevertheless, if that is not standard practice we should not do this approach.

@vjeffrey I propose we leave the handler files there for now, I've an idea how we can get around generating the file in future to get the best of both worlds. But lets try that in future PRs.

cool, sounds good! also, get some sleep @chris-rock!!

just updated by dropping that last commit. :) thanks for your help @jeremymv2!! good points!

Signed-off-by: Christoph Hartmann <chris@lollyrock.com>

binamov added the in progress label Oct 17, 2016

use chef handler to run inspec tests

43845a1

vjeffrey force-pushed the audit-2.0 branch from a26cc07 to 5ff0c94 Compare October 19, 2016 01:04

setting up visibility reporting. almost there.

182ff45

vjeffrey force-pushed the audit-2.0 branch 2 times, most recently from 12e7dd4 to 5c6b12d Compare October 19, 2016 02:01

fixing lint errors. most tests fixed.

d6eef1d

Signed-off-by: Victoria Jeffrey <vjeffrey@chef.io>

vjeffrey force-pushed the audit-2.0 branch from 5c6b12d to d6eef1d Compare October 19, 2016 02:02

Victoria Jeffrey added 3 commits October 19, 2016 12:48

use chef handler to run inspec tests

a42530a

setting up visibility reporting. almost there.

c93b3d0

fixing lint errors. most tests fixed.

c730ead

Signed-off-by: Victoria Jeffrey <vjeffrey@chef.io>

chris-rock force-pushed the audit-2.0 branch from d6eef1d to c730ead Compare October 19, 2016 10:48

chris-rock reviewed Oct 19, 2016

View reviewed changes

recover examples

e260c52

Signed-off-by: Christoph Hartmann <chris@lollyrock.com>

chris-rock force-pushed the audit-2.0 branch from 53d9135 to e260c52 Compare October 19, 2016 11:53

Merge branch 'audit-2.0' of github.com:chef-cookbooks/audit into audi…

7ebcb8f

…t-2.0

chris-rock reviewed Oct 19, 2016

View reviewed changes

chris-rock mentioned this pull request Oct 19, 2016

Activate test-kitchen in travis #114

Merged

chris-rock added this to the 2.0 milestone Oct 19, 2016

jeremymv2 reviewed Oct 19, 2016

View reviewed changes

vjeffrey force-pushed the audit-2.0 branch 2 times, most recently from 1b94343 to 087609e Compare October 19, 2016 16:58

vjeffrey changed the title ~~wip: use chef handler to run inspec tests~~ use chef handler to run inspec tests Oct 19, 2016

jeremymv2 reviewed Oct 19, 2016

View reviewed changes

vjeffrey force-pushed the audit-2.0 branch from 12240c3 to 087609e Compare October 20, 2016 02:02

chris-rock approved these changes Oct 20, 2016

View reviewed changes

readd version attribute for inspec gem

2ed2b59

Signed-off-by: Christoph Hartmann <chris@lollyrock.com>

vjeffrey force-pushed the audit-2.0 branch from eee1792 to 2ed2b59 Compare October 20, 2016 19:56

vjeffrey merged commit aed623f into master Oct 20, 2016

iennae removed the in progress label Oct 20, 2016

vjeffrey deleted the audit-2.0 branch October 20, 2016 20:04

sbabcoc mentioned this pull request Jul 18, 2019

Implement an 'audit' reporter that will terminate Chef Client runs on profile failures #380

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use chef handler to run inspec tests #113

use chef handler to run inspec tests #113

vjeffrey commented Oct 17, 2016

vjeffrey commented Oct 18, 2016 •

edited

Loading

vjeffrey commented Oct 19, 2016

chris-rock Oct 19, 2016

chris-rock Oct 19, 2016

chris-rock Oct 19, 2016

chris-rock Oct 19, 2016

vjeffrey Oct 19, 2016

chris-rock Oct 19, 2016

vjeffrey Oct 19, 2016

vjeffrey Oct 19, 2016

chris-rock Oct 19, 2016

chris-rock Oct 19, 2016

vjeffrey Oct 19, 2016

jeremymv2 Oct 19, 2016

vjeffrey Oct 19, 2016

vjeffrey commented Oct 19, 2016

jeremymv2 commented Oct 19, 2016

vjeffrey commented Oct 19, 2016

jeremymv2 Oct 19, 2016 •

edited

Loading

chris-rock Oct 19, 2016 •

edited

Loading

vjeffrey Oct 20, 2016

vjeffrey Oct 20, 2016 •

edited

Loading

		@@ -46,11 +48,9 @@
		# fail converge after posting report if any audits have failed
		default['audit']['fail_if_any_audits_failed'] = false

use chef handler to run inspec tests #113

use chef handler to run inspec tests #113

Conversation

vjeffrey commented Oct 17, 2016

vjeffrey commented Oct 18, 2016 • edited Loading

vjeffrey commented Oct 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vjeffrey commented Oct 19, 2016

jeremymv2 commented Oct 19, 2016

vjeffrey commented Oct 19, 2016

jeremymv2 Oct 19, 2016 • edited Loading

Choose a reason for hiding this comment

chris-rock Oct 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vjeffrey Oct 20, 2016 • edited Loading

Choose a reason for hiding this comment

vjeffrey commented Oct 18, 2016 •

edited

Loading

jeremymv2 Oct 19, 2016 •

edited

Loading

chris-rock Oct 19, 2016 •

edited

Loading

vjeffrey Oct 20, 2016 •

edited

Loading